Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix userinfo response unwrapping for discord provider #2254

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

egans146
Copy link

This fix unwrap the discord userinfo response allowing OpenIddict mapping to the corresponding claims

@kevinchalet
Copy link
Member

Thanks for your PR, @egans146!

It's something I also noticed but since it's technically a breaking change, I wasn't sure if it was the right thing to do.

Since you decided to send a PR, I guess the existing logic (i.e returning the bot+user claims unflatten) is not ideal. On the other hand, if we do that, the bot claims will no longer be returned. A middle ground could be to flatten the user claims and still include a bot claims node as we do for some of the other providers. What do you think?

@egans146
Copy link
Author

Thank you for taking the time to respond.

I think your proposal would be a good middle ground. I may have initially rushed and failed to consider the other nodes that would no longer be returned.

You think the « bot » node nested in « application » should be included or the whole application node ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants